Renew golden token indexing #279
Conversation
- remove realms apollo client context - add blastapi nftcollection query
- fix dependencies for golden tokens fetch
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Warning Rate limit exceeded@starknetdev has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 8 minutes and 20 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThis pull request introduces a new asynchronous function Changes
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Outside diff range and nitpick comments (11)
ui/src/app/hooks/useCustomQuery.ts (2)
Line range hint
24-27: LGTM! Consider adding a clarifying comment.The removal of the
clientoption from theuseQuerycall is consistent with the changes in imports. The hook now uses the default Apollo Client instance.Consider adding a comment to explain the use of the default Apollo Client:
const { data, refetch } = useQuery(query, { variables: variables, skip: skip, + // Using the default Apollo Client instance });
Line range hint
1-43: Summary: Simplified Apollo Client usage in custom hook.The changes in this file streamline the
useCustomQueryhook by removing the custom Apollo Client creation. This modification is likely part of a larger refactoring effort to centralize Apollo Client configuration, which should improve maintainability and consistency across the application.Key points:
- Removed custom Apollo Client creation
- Now using the default Apollo Client instance
- Overall hook functionality remains unchanged
These changes appear to be an improvement in the code structure. However, ensure that the global Apollo Client setup is properly configured to handle different networks as needed by this hook.
Consider documenting the new Apollo Client setup in the project's README or a separate GraphQL documentation file to help other developers understand the centralized configuration approach.
ui/src/app/components/start/CreateAdventurer.tsx (2)
12-12: LGTM! Consider updating documentation.The changes to the
CreateAdventurerPropsinterface look good. The newgoldenTokensproperty replaces the previouslordsBalanceandgoldenTokenData, which seems to be a more straightforward approach to handling golden token data.Consider updating any relevant documentation or comments to reflect these changes, especially regarding the removal of
mintLordsfunctionality and the new structure of golden token data.
Line range hint
1-150: Overall changes look good. Consider broader impact analysis.The modifications to the
CreateAdventurercomponent and its related interfaces improve the handling of golden token data by introducing a more straightforwardgoldenTokensprop. The changes are consistent throughout the file.Consider conducting a broader impact analysis to ensure that:
- The removal of
mintLordsfunctionality doesn't negatively impact other parts of the application.- Any components or utilities that previously relied on
lordsBalanceorgoldenTokenDatahave been updated accordingly.- The new
goldenTokensdata structure is compatible with the rest of the application's state management and API interactions.This will help maintain the overall consistency and functionality of the application.
ui/src/app/containers/AdventurerScreen.tsx (1)
28-28: Improved type safety forgoldenTokensThe change from
goldenTokenData: any;togoldenTokens: number[];enhances type safety and clarity. This is a positive improvement.Consider adding a brief comment explaining the purpose of
goldenTokensfor better code documentation:/** Array of golden token IDs associated with the adventurer */ goldenTokens: number[];ui/src/app/components/start/Spawn.tsx (3)
44-48: Approved: Updated prop destructuring in Spawn componentThe changes to the prop destructuring in the
Spawncomponent correctly reflect the updates made to theSpawnPropsinterface. The removal oflordsBalanceandgoldenTokenData, and the addition ofgoldenTokensalign with the new data structure for golden tokens.Consider renaming
goldenTokensto something more specific, such asgoldenTokenIds, to clearly indicate that it's an array of token IDs. This would improve code readability and make the purpose of the prop more explicit.- goldenTokens, + goldenTokenIds,
127-127: Approved: Updated getUsableGoldenToken function callThe change to use
goldenTokens ?? []in thegetUsableGoldenTokenfunction call is a good improvement. It simplifies the token retrieval process and aligns with the new data structure while providing a fallback empty array to prevent potential errors.Consider adding a console warning or error logging when
goldenTokensis undefined, as this might indicate an unexpected state in the application:- getUsableGoldenToken(goldenTokens ?? []); + if (goldenTokens === undefined) { + console.warn('goldenTokens is undefined in Spawn component'); + } + getUsableGoldenToken(goldenTokens ?? []);This addition would help with debugging and identifying potential issues in the data flow.
⚠️ Incomplete Refactoring of Golden Token HandlingThe refactoring to replace
goldenTokenDatawithgoldenTokensin theSpawncomponent has been partially successful. WhileSpawn.tsxno longer referencesgoldenTokenData, there are still instances ofgoldenTokenDatainui/src/app/api/getGoldenTokens.ts. Additionally,goldenTokensis correctly used in multiple files, includingSpawn.tsx,page.tsx,CreateAdventurer.tsx, andAdventurerScreen.tsx.Action Items:
Update
getGoldenTokens.ts:
- Remove or replace
goldenTokenDatawithgoldenTokens.- Ensure that the data fetching and handling logic aligns with the new structure.
Verify Consistency Across the Codebase:
- Ensure no other files reference
goldenTokenData.- Update any related unit tests to reflect these changes.
🔗 Analysis chain
Line range hint
1-438: Summary: Successful refactoring of golden token handlingThe changes made to the
Spawncomponent successfully refactor the handling of golden tokens. The modifications are well-contained and focused on the specific areas that needed updating. The transition from usinggoldenTokenDatato a simplergoldenTokensarray appears to be seamless and doesn't disrupt the overall functionality of the component.These changes should improve the component's maintainability and type safety. However, to ensure the refactoring hasn't introduced any regressions, consider the following:
- Update any unit tests associated with this component to reflect the new prop structure.
- Perform thorough manual testing of the golden token functionality to verify it works as expected with the new data structure.
- Review any parent components or contexts that provide the golden token data to ensure they're now passing an array of numbers instead of the previous data structure.
To help with this verification, you can run the following script to check for any remaining references to the old data structure:
This will help ensure that the refactoring has been applied consistently across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of goldenTokenData in the codebase echo "Checking for goldenTokenData usage:" rg --type typescript 'goldenTokenData' echo "Checking for new goldenTokens usage:" rg --type typescript 'goldenTokens'Length of output: 308
Script:
#!/bin/bash # Description: Recheck for any remaining usage of goldenTokenData in the codebase with corrected file type echo "Rechecking for goldenTokenData usage:" rg --type tsx 'goldenTokenData' echo "Rechecking for new goldenTokens usage:" rg --type tsx 'goldenTokens'Length of output: 288
Script:
#!/bin/bash # Description: Recheck for any remaining usage of goldenTokenData and verify new goldenTokens usage in .ts and .tsx files echo "Rechecking for goldenTokenData usage in .ts files:" rg --glob '*.ts' 'goldenTokenData' echo "Rechecking for goldenTokenData usage in .tsx files:" rg --glob '*.tsx' 'goldenTokenData' echo "Rechecking for new goldenTokens usage in .ts files:" rg --glob '*.ts' 'goldenTokens' echo "Rechecking for new goldenTokens usage in .tsx files:" rg --glob '*.tsx' 'goldenTokens'Length of output: 2038
ui/src/app/page.tsx (2)
97-97: Golden tokens state and fetching logic added.The implementation for fetching and storing golden tokens is well-structured. The state is correctly initialized, and the useEffect hook ensures that the tokens are fetched when necessary.
However, consider adding error handling to the
handleFetchGoldenTokensfunction:const handleFetchGoldenTokens = async () => { + try { const goldenTokens = await getGoldenTokens( address ?? "", networkConfig[network!].goldenTokenAddress, network ); setGoldenTokens(goldenTokens); + } catch (error) { + console.error("Failed to fetch golden tokens:", error); + // Optionally, update UI to show error state + } };This will ensure that any errors during the fetch process are properly logged and handled.
Also applies to: 470-481
882-882: Golden tokens prop added to AdventurerScreen component.The
goldenTokensstate is correctly passed as a prop to the AdventurerScreen component, allowing it to access and display the golden tokens data.For improved type safety, consider adding a prop type for
goldenTokensin the AdventurerScreen component:interface AdventurerScreenProps { // ... other props goldenTokens: number[]; }This will ensure that the correct type of data is always passed to the component.
ui/src/app/api/getGoldenTokens.ts (1)
24-26: Remove unnecessaryContent-Typeheader for GET requestThe
Content-Typeheader is typically used to indicate the media type of the request body in POST or PUT requests. It is unnecessary for a GET request with no body. Consider removing it.Apply this diff to remove the unnecessary header:
const response = await fetch(url, { method: "GET", - headers: { - "Content-Type": "application/json", - }, });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
- ui/src/app/api/getGoldenTokens.ts (1 hunks)
- ui/src/app/components/start/CreateAdventurer.tsx (3 hunks)
- ui/src/app/components/start/Spawn.tsx (3 hunks)
- ui/src/app/containers/AdventurerScreen.tsx (3 hunks)
- ui/src/app/hooks/useCustomQuery.ts (1 hunks)
- ui/src/app/layout.tsx (2 hunks)
- ui/src/app/lib/networkConfig.ts (4 hunks)
- ui/src/app/page.tsx (5 hunks)
🧰 Additional context used
🔇 Additional comments (13)
ui/src/app/hooks/useCustomQuery.ts (1)
1-4: LGTM! Verify global Apollo Client setup.The changes to the import statements look good. The removal of
gameClientandnetworkConfigimports suggests a shift towards using a global Apollo Client instance.Please confirm that a global Apollo Client instance is properly set up elsewhere in the application. Run the following script to verify:
✅ Verification successful
Verified: Global Apollo Client is correctly set up.
The global Apollo Client instance is properly initialized inui/src/app/lib/clients.ts. No further action is required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for Apollo Client setup in the application # Test: Search for Apollo Client setup rg --type typescript -e "new ApolloClient" -e "createClient" -e "makeApolloClient"Length of output: 115
Script:
#!/bin/bash # Description: Check for Apollo Client setup in the application # Test: Search for Apollo Client setup in TypeScript files rg -g "*.ts" -g "*.tsx" -e "new ApolloClient" -e "createClient" -e "makeApolloClient"Length of output: 191
ui/src/app/components/start/CreateAdventurer.tsx (2)
23-23: LGTM! Props updated correctly.The changes to the destructured props in the
CreateAdventurercomponent correctly reflect the updates made to theCreateAdventurerPropsinterface. The addition ofgoldenTokensand the removal oflordsBalance,goldenTokenData, andmintLordsare consistent with the new data structure.
118-122: LGTM! Verify Spawn component implementation.The updates to the
Spawncomponent props are consistent with the changes made to theCreateAdventurercomponent and its interface. Both instances of theSpawncomponent have been updated correctly.Please ensure that the
Spawncomponent has been updated to handle the newgoldenTokensprop correctly and that it no longer relies on the removedlordsBalance,goldenTokenData, andmintLordsprops. Run the following script to verify theSpawncomponent implementation:Also applies to: 142-146
✅ Verification successful
Please run the following shell scripts to verify the
Spawncomponent implementation:
Verified! The
Spawncomponent correctly uses thegoldenTokensprop and no longer relies onlordsBalance,goldenTokenData, ormintLords.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify Spawn component implementation # Test: Check if Spawn component is using goldenTokens prop and not using removed props ast-grep --lang typescript --pattern $'function Spawn({ $$$, goldenTokens, $$$ }: SpawnProps) { $$$ }' # Test: Ensure lordsBalance, goldenTokenData, and mintLords are not used in Spawn rg --type typescript 'lordsBalance|goldenTokenData|mintLords' $(fd -e tsx -e ts)Length of output: 8325
Script:
#!/bin/bash # Description: Verify Spawn component implementation # Test: Check if Spawn component is using goldenTokens prop and matches the expected pattern ast-grep --lang typescript --pattern $'function Spawn({ $$$, goldenTokens, $$$ }: SpawnProps) { $$$ }' # Test: Ensure lordsBalance, goldenTokenData, and mintLords are not used in Spawn rg 'lordsBalance|goldenTokenData|mintLords' -- '*.ts' '*.tsx'Length of output: 260
Script:
#!/bin/bash # Description: Verify Spawn component implementation # Test: Check if Spawn component is using goldenTokens prop and matches the expected pattern ast-grep --lang typescript --pattern $'const Spawn = ({ $$$, goldenTokens, $$$ }: SpawnProps) => { $$$ }' # Test: Ensure lordsBalance, goldenTokenData, and mintLords are not used in Spawn rg 'lordsBalance|goldenTokenData|mintLords' --glob='*.ts' --glob='*.tsx'Length of output: 3817
ui/src/app/containers/AdventurerScreen.tsx (2)
48-48: Props updated to match interface changesThe addition of
goldenTokensto the destructured props aligns with the interface changes. This is correct and maintains consistency.To ensure all necessary changes have been made, please verify the removal of
lordsBalanceandmintLordsfrom the props. Run the following script to check for any remaining references:✅ Verification successful
Removed
lordsBalanceandmintLordsprops properlyNo remaining references to
lordsBalanceandmintLordswere found inAdventurerScreen.tsx. The props have been successfully removed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to removed props # Test: Search for lordsBalance and mintLords in the file rg --type typescript 'lordsBalance|mintLords' ui/src/app/containers/AdventurerScreen.tsxLength of output: 125
Script:
#!/bin/bash # Description: Check for any remaining references to removed props # Test: Search for lordsBalance and mintLords in the file rg --type tsx 'lordsBalance|mintLords' ui/src/app/containers/AdventurerScreen.tsxLength of output: 111
Script:
#!/bin/bash # Description: Check for any remaining references to removed props # Test: Search for lordsBalance and mintLords in the file without specifying file type rg 'lordsBalance|mintLords' ui/src/app/containers/AdventurerScreen.tsxLength of output: 72
136-140: Props correctly passed toCreateAdventurerThe
goldenTokensprop is now correctly passed to theCreateAdventurercomponent, aligning with the interface changes. This ensures that the child component receives the updated data structure.To ensure the
CreateAdventurercomponent is using the newgoldenTokensprop correctly and has removed any references to the oldgoldenTokenData,lordsBalance, andmintLordsprops, run the following script:Ensure that the
CreateAdventurercomponent's props and implementation have been updated to match these changes.ui/src/app/lib/networkConfig.ts (5)
78-78: Clarify the emptyblastUrlfor katana configurationThe
blastUrlproperty has been added to the katana configuration, but it's set to an empty string. Could you please clarify:
- Is this intentional, or should there be a specific URL for the katana environment?
- If intentional, are there any plans to implement Blast API support for katana in the future?
This information will help ensure consistency across different environments and prevent potential issues in the future.
107-107: Clarify the emptyblastUrlfor localKatana configurationSimilar to the katana configuration, the
blastUrlproperty for localKatana is set to an empty string. Please provide clarification on this:
- Is this intentional for the local development environment?
- If so, how should the application behave when encountering an empty
blastUrl?- Are there any specific considerations or setup steps for local development regarding the Blast API integration?
This information will help ensure smooth local development and testing processes.
Line range hint
1-135: Summary of networkConfig.ts changesThe changes in this file align with the PR objective of transitioning to Blast APIs for golden token indexing. Here's a summary of the key points:
blastUrlhas been added to all environment configurations.- The sepolia configuration uses a mainnet URL, which needs verification.
- The katana and localKatana configurations have empty
blastUrlvalues, requiring clarification.- The removal of
tokensGQLURLis not visible in the diff but is mentioned in the summary and needs confirmation.Please address the points raised in the individual comments to ensure consistency and clarity across all environment configurations.
#!/bin/bash # Verify the presence and usage of blastUrl and tokensGQLURL echo "Occurrences of blastUrl:" grep -n "blastUrl" ui/src/app/lib/networkConfig.ts echo "\nOccurrences of tokensGQLURL:" grep -n "tokensGQLURL" ui/src/app/lib/networkConfig.ts echo "\nCheck for any TODO comments related to these changes:" grep -n "TODO" ui/src/app/lib/networkConfig.ts
6-7: Verify theblastUrlfor sepolia configurationThe
blastUrlhas been added, which aligns with the PR objective of transitioning to Blast APIs. However, the URL appears to be for the mainnet environment, not sepolia. Please verify if this is intentional or if it should be updated to a sepolia-specific endpoint.Also, confirm if the removal of
tokensGQLURL(not visible in this diff) is intentional and consistent with the new API usage.
39-40: LGTM! Verify removal oftokensGQLURLThe addition of
blastUrlfor the mainnet configuration is correct and aligns with the PR objectives. The URL is appropriate for the mainnet environment.Please confirm that the
tokensGQLURLproperty has been removed from this configuration as well, as it's not visible in this diff but mentioned in the summary.✅ Verification successful
Verified! The
tokensGQLURLproperty has been successfully removed from the mainnet configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if tokensGQLURL has been removed from mainnet config grep -n "tokensGQLURL" ui/src/app/lib/networkConfig.tsLength of output: 479
ui/src/app/page.tsx (2)
7-7: New import for golden tokens functionality added.The
getGoldenTokensfunction is now imported from the API, which is a good practice for modularizing the code and separating concerns.
Line range hint
1-938: Golden tokens feature successfully integrated.The changes to integrate the golden tokens functionality are well-implemented. The code maintains good structure and follows React best practices. The new feature is properly integrated with existing state management and component structure.
Key points:
- New import for
getGoldenTokensfunction.- State management for golden tokens with proper fetching logic.
- Integration with the AdventurerScreen component.
The suggestions for error handling and type safety will further improve the robustness of the implementation. Overall, this is a solid addition to the main page component.
ui/src/app/layout.tsx (1)
86-90: Verify the Nesting Order of Context ProvidersThe context providers are nested as follows:
<ApolloProvider client={gameClient(networkConfig[network].lsGQLURL!)}> <ControllerProvider> <StarknetProvider network={network}> <DojoProvider value={setupResult}>{children}</DojoProvider> </StarknetProvider> </ControllerProvider> </ApolloProvider>Ensure that the order of these providers aligns with their dependencies. If
ControllerProviderdepends onStarknetProvideror any other provider, nesting them incorrectly could lead to context issues within child components.To verify, check if any context consumers inside
ControllerProviderrequire the contexts provided byStarknetProviderorDojoProvider. If so, adjust the nesting order accordingly:<ApolloProvider client={gameClient(networkConfig[network].lsGQLURL!)}> <StarknetProvider network={network}> <ControllerProvider> <DojoProvider value={setupResult}>{children}</DojoProvider> </ControllerProvider> </StarknetProvider> </ApolloProvider>
| @@ -13,7 +13,7 @@ import useUIStore from "@/app/hooks/useUIStore"; | |||
| import { battle } from "@/app/lib/constants"; | |||
| import { networkConfig } from "@/app/lib/networkConfig"; | |||
| import { formatLords } from "@/app/lib/utils"; | |||
| import { Adventurer, FormData, GameToken } from "@/app/types"; | |||
| import { Adventurer, FormData } from "@/app/types"; | |||
There was a problem hiding this comment.
💡 Codebase verification
Action Required: Remaining References to lordsBalance and goldenTokenData Detected
The following files still reference lordsBalance and goldenTokenData, which need to be updated to align with the recent SpawnProps modifications:
-
lordsBalance:
ui/src/app/page.tsxui/src/app/lib/utils/syscalls.tsui/src/app/lib/balances.tsui/src/app/containers/TopUp.tsxui/src/app/containers/Onboarding.tsxui/src/app/components/profile/ProfileDialog.tsxui/src/app/components/navigation/Header.tsx
-
goldenTokenData:
ui/src/app/api/getGoldenTokens.ts
-
goldenTokens:
ui/src/app/page.tsxui/src/app/containers/AdventurerScreen.tsxui/src/app/components/start/CreateAdventurer.tsxui/src/app/components/start/Spawn.tsx
Please update these references to ensure consistency and type safety across the codebase.
🔗 Analysis chain
Approved: Updated imports and SpawnProps interface
The changes to the imports and SpawnProps interface look good. The addition of Adventurer and FormData types import suggests improved type safety. The modification of SpawnProps to use goldenTokens: number[] instead of goldenTokenData: any is a positive change, as it provides more specific typing and potentially simplifies the component's logic.
To ensure this change is consistently applied throughout the codebase, please run the following script:
Also applies to: 33-37
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining usage of lordsBalance or goldenTokenData in SpawnProps
# Search for lordsBalance in SpawnProps
echo "Checking for lordsBalance in SpawnProps:"
rg --type typescript 'SpawnProps.*lordsBalance'
# Search for goldenTokenData in SpawnProps
echo "Checking for goldenTokenData in SpawnProps:"
rg --type typescript 'SpawnProps.*goldenTokenData'
# Search for usage of goldenTokens in SpawnProps
echo "Checking for goldenTokens in SpawnProps:"
rg --type typescript 'SpawnProps.*goldenTokens'
Length of output: 536
Script:
#!/bin/bash
# Description: Check for any remaining usage of lordsBalance or goldenTokenData in SpawnProps
# Define the search patterns
PATTERNS=("lordsBalance" "goldenTokenData")
# Iterate over each pattern and search in .ts and .tsx files
for pattern in "${PATTERNS[@]}"; do
echo "Checking for ${pattern} in SpawnProps:"
rg "${pattern}" --glob "*.ts" --glob "*.tsx"
done
# Additionally, check for usage of goldenTokens in SpawnProps
echo "Checking for goldenTokens in SpawnProps:"
rg "goldenTokens" --glob "*.ts" --glob "*.tsx"
Length of output: 3865
ui/src/app/api/getGoldenTokens.ts
Outdated
| let url = `${ | ||
| networkConfig[network!].blastUrl | ||
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; |
There was a problem hiding this comment.
Avoid using non-null assertion operator; ensure network is defined
Using the non-null assertion operator (!) can mask potential null or undefined errors. Instead, consider adding a runtime check to ensure that network is defined or handle cases where it might be undefined.
Apply this diff to add a check:
+ if (!network) {
+ throw new Error("Network is not defined");
+ }
let url = `${
- networkConfig[network!].blastUrl
+ networkConfig[network].blastUrl
}/builder/getWalletNFTs?contractAddress=...`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let url = `${ | |
| networkConfig[network!].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; | |
| if (!network) { | |
| throw new Error("Network is not defined"); | |
| } | |
| let url = `${ | |
| networkConfig[network].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; |
URL-encode query parameters to prevent injection attacks
Directly interpolating user-provided values into a URL can lead to injection vulnerabilities. Use encodeURIComponent to safely encode goldenTokenAddress and owner parameters.
Apply this diff to encode the parameters:
let url = `${
networkConfig[network!].blastUrl
}/builder/getWalletNFTs?contractAddress=${
- goldenTokenAddress
+ encodeURIComponent(goldenTokenAddress)
}&walletAddress=${
- owner
+ encodeURIComponent(owner)
}&pageSize=100`;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let url = `${ | |
| networkConfig[network!].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; | |
| let url = `${ | |
| networkConfig[network!].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${ | |
| encodeURIComponent(goldenTokenAddress) | |
| }&walletAddress=${ | |
| encodeURIComponent(owner) | |
| }&pageSize=100`; |
| const recursiveFetch: any = async ( | ||
| goldenTokens: any[], | ||
| nextPageKey: string | null | ||
| ) => { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Specify explicit types instead of using any
Using any defeats TypeScript's static type checking. Consider defining explicit types for the recursiveFetch function and its parameters to enhance type safety and maintainability.
Apply this diff to specify explicit types:
- const recursiveFetch: any = async (
- goldenTokens: any[],
- nextPageKey: string | null
- ) => {
+ const recursiveFetch = async (
+ goldenTokens: number[],
+ nextPageKey: string | null
+ ): Promise<number[]> => {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const recursiveFetch: any = async ( | |
| goldenTokens: any[], | |
| nextPageKey: string | null | |
| ) => { | |
| const recursiveFetch = async ( | |
| goldenTokens: number[], | |
| nextPageKey: string | null | |
| ): Promise<number[]> => { |
| const data = await response.json(); | ||
| goldenTokens = goldenTokens.concat( | ||
| data?.nfts?.map((goldenToken: any) => { | ||
| const tokenId = JSON.parse(goldenToken.tokenId); | ||
| return Number(tokenId); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
Handle cases where data.nfts might be undefined
If data.nfts is undefined or null, mapping over it will result in an error. Consider adding a check to ensure data.nfts is an array before mapping over it.
Apply this diff to add a default empty array if data.nfts is undefined:
const data = await response.json();
goldenTokens = goldenTokens.concat(
- data?.nfts?.map((goldenToken: any) => {
+ (data?.nfts ?? []).map((goldenToken: any) => {
const tokenId = Number(goldenToken.tokenId);
return tokenId;
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const data = await response.json(); | |
| goldenTokens = goldenTokens.concat( | |
| data?.nfts?.map((goldenToken: any) => { | |
| const tokenId = JSON.parse(goldenToken.tokenId); | |
| return Number(tokenId); | |
| }) | |
| ); | |
| const data = await response.json(); | |
| goldenTokens = goldenTokens.concat( | |
| (data?.nfts ?? []).map((goldenToken: any) => { | |
| const tokenId = JSON.parse(goldenToken.tokenId); | |
| return Number(tokenId); | |
| }) | |
| ); |
| data?.nfts?.map((goldenToken: any) => { | ||
| const tokenId = JSON.parse(goldenToken.tokenId); | ||
| return Number(tokenId); | ||
| }) |
There was a problem hiding this comment.
Avoid unnecessary JSON.parse on goldenToken.tokenId
Using JSON.parse on goldenToken.tokenId may be unnecessary and could lead to errors if tokenId is not valid JSON. If goldenToken.tokenId is a string representation of a number, you can convert it directly using Number(goldenToken.tokenId).
Apply this diff to simplify the code:
const data = await response.json();
goldenTokens = goldenTokens.concat(
(data?.nfts ?? []).map((goldenToken: any) => {
- const tokenId = JSON.parse(goldenToken.tokenId);
+ const tokenId = Number(goldenToken.tokenId);
return tokenId;
})
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| data?.nfts?.map((goldenToken: any) => { | |
| const tokenId = JSON.parse(goldenToken.tokenId); | |
| return Number(tokenId); | |
| }) | |
| data?.nfts?.map((goldenToken: any) => { | |
| const tokenId = Number(goldenToken.tokenId); | |
| return tokenId; | |
| }) |
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); | ||
|
|
||
| const data = await response.json(); |
There was a problem hiding this comment.
Add response status check before parsing JSON
Currently, the code assumes that the fetch request is successful and proceeds to parse the response JSON. To handle potential HTTP errors, check response.ok and handle error statuses appropriately before calling response.json().
Apply this diff to add a response status check:
try {
const response = await fetch(url, {
method: "GET",
// headers...
});
+ if (!response.ok) {
+ throw new Error(`HTTP error! Status: ${response.status}`);
+ }
const data = await response.json();
// Process data...Committable suggestion was skipped due to low confidence.
ui/src/app/api/getGoldenTokens.ts
Outdated
| const recursiveFetch: any = async ( | ||
| goldenTokens: any[], | ||
| nextPageKey: string | null | ||
| ) => { | ||
| let url = `${ | ||
| networkConfig[network!].blastUrl | ||
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; | ||
|
|
||
| if (nextPageKey) { | ||
| url += `&pageKey=${nextPageKey}`; | ||
| } | ||
|
|
||
| try { | ||
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
| goldenTokens = goldenTokens.concat( | ||
| data?.nfts?.map((goldenToken: any) => { | ||
| const tokenId = JSON.parse(goldenToken.tokenId); | ||
| return Number(tokenId); | ||
| }) | ||
| ); | ||
|
|
||
| if (data.nextPageKey) { | ||
| return recursiveFetch(goldenTokens, data.nextPageKey); | ||
| } | ||
| } catch (ex) { | ||
| console.log("error fetching golden tokens", ex); | ||
| } | ||
|
|
||
| return goldenTokens; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor recursion to iteration for fetching data
Using recursion for fetching paginated data can lead to stack overflow errors if the number of pages is large. An iterative approach is more reliable and efficient.
Apply this diff to refactor to an iterative approach:
- const recursiveFetch = async (
- goldenTokens: number[],
- nextPageKey: string | null
- ): Promise<number[]> => {
- // Recursive fetching logic...
- };
-
- let goldenTokenData = await recursiveFetch([], null);
+ const fetchAllPages = async (): Promise<number[]> => {
+ let goldenTokens: number[] = [];
+ let nextPageKey: string | null = null;
+ do {
+ let url = `${
+ networkConfig[network].blastUrl
+ }/builder/getWalletNFTs?contractAddress=${encodeURIComponent(goldenTokenAddress)}&walletAddress=${encodeURIComponent(owner)}&pageSize=100`;
+ if (nextPageKey) {
+ url += `&pageKey=${nextPageKey}`;
+ }
+ try {
+ const response = await fetch(url, {
+ method: "GET",
+ });
+ if (!response.ok) {
+ throw new Error(`HTTP error! Status: ${response.status}`);
+ }
+ const data = await response.json();
+ goldenTokens = goldenTokens.concat(
+ (data?.nfts ?? []).map((goldenToken: any) => {
+ const tokenId = Number(goldenToken.tokenId);
+ return tokenId;
+ })
+ );
+ nextPageKey = data.nextPageKey || null;
+ } catch (ex) {
+ console.log("error fetching golden tokens", ex);
+ break;
+ }
+ } while (nextPageKey);
+ return goldenTokens;
+ };
+
+ let goldenTokenData = await fetchAllPages();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const recursiveFetch: any = async ( | |
| goldenTokens: any[], | |
| nextPageKey: string | null | |
| ) => { | |
| let url = `${ | |
| networkConfig[network!].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${owner}&pageSize=100`; | |
| if (nextPageKey) { | |
| url += `&pageKey=${nextPageKey}`; | |
| } | |
| try { | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| }); | |
| const data = await response.json(); | |
| goldenTokens = goldenTokens.concat( | |
| data?.nfts?.map((goldenToken: any) => { | |
| const tokenId = JSON.parse(goldenToken.tokenId); | |
| return Number(tokenId); | |
| }) | |
| ); | |
| if (data.nextPageKey) { | |
| return recursiveFetch(goldenTokens, data.nextPageKey); | |
| } | |
| } catch (ex) { | |
| console.log("error fetching golden tokens", ex); | |
| } | |
| return goldenTokens; | |
| }; | |
| const fetchAllPages = async (): Promise<number[]> => { | |
| let goldenTokens: number[] = []; | |
| let nextPageKey: string | null = null; | |
| do { | |
| let url = `${ | |
| networkConfig[network].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${encodeURIComponent(goldenTokenAddress)}&walletAddress=${encodeURIComponent(owner)}&pageSize=100`; | |
| if (nextPageKey) { | |
| url += `&pageKey=${nextPageKey}`; | |
| } | |
| try { | |
| const response = await fetch(url, { | |
| method: "GET", | |
| }); | |
| if (!response.ok) { | |
| throw new Error(`HTTP error! Status: ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| goldenTokens = goldenTokens.concat( | |
| (data?.nfts ?? []).map((goldenToken: any) => { | |
| const tokenId = Number(goldenToken.tokenId); | |
| return tokenId; | |
| }) | |
| ); | |
| nextPageKey = data.nextPageKey || null; | |
| } catch (ex) { | |
| console.log("error fetching golden tokens", ex); | |
| break; | |
| } | |
| } while (nextPageKey); | |
| return goldenTokens; | |
| }; | |
| let goldenTokenData = await fetchAllPages(); |
| import BurnerLoader from "@/app/components/animations/BurnerLoader"; | ||
| import Intro from "@/app/components/intro/Intro"; | ||
| import LoginIntro from "@/app/components/onboarding/Intro"; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Clarify Component Naming to Avoid Confusion
The imports on lines 3-5 include both Intro and LoginIntro components:
import BurnerLoader from "@/app/components/animations/BurnerLoader";
import Intro from "@/app/components/intro/Intro";
import LoginIntro from "@/app/components/onboarding/Intro";Having components with similar names can lead to confusion and maintainability issues. Consider renaming LoginIntro to a more descriptive name that reflects its specific purpose, or organizing them in a way that distinguishes their roles clearly.
| import Head from "@/app/head"; | ||
| import useUIStore from "@/app/hooks/useUIStore"; | ||
| import { gameClient } from "@/app/lib/clients"; |
There was a problem hiding this comment.
Ensure lsGQLURL is Defined for All Networks
On line 12, the gameClient is imported:
import { gameClient } from "@/app/lib/clients";Later, in the ApolloProvider, gameClient is called with:
<ApolloProvider client={gameClient(networkConfig[network].lsGQLURL!)}>The exclamation mark ! asserts that lsGQLURL is not undefined. To prevent potential runtime errors, verify that lsGQLURL is defined for all possible values of network in networkConfig. If there's a chance that lsGQLURL could be undefined, include appropriate error handling or validation before using it.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- ui/src/app/api/getGoldenTokens.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (3)
ui/src/app/api/getGoldenTokens.ts (3)
1-9: LGTM: Function signature and imports are well-defined.The function signature for
getGoldenTokensis properly typed, and the necessary imports are present. This provides a good foundation for the implementation.
50-53: Consider updating main function execution if refactoredThe current implementation is correct for the recursive approach. However, if you refactor to an iterative approach as suggested earlier, update this section accordingly.
1-53: Summary of review for getGoldenTokens.tsThe
getGoldenTokensfunction provides a solid foundation for fetching golden tokens, but several improvements can enhance its robustness and efficiency:
- Improve type safety by replacing
anytypes with explicit types.- Enhance error handling and add proper checks for undefined values.
- Implement URL encoding for query parameters to prevent injection attacks.
- Consider refactoring the recursive approach to an iterative one for better efficiency and to avoid potential stack overflow issues with large datasets.
Addressing these points will significantly improve the function's reliability and maintainability.
| const data = await response.json(); | ||
| goldenTokens = goldenTokens.concat( | ||
| data?.nfts?.map((goldenToken: any) => { | ||
| const tokenId = JSON.parse(goldenToken.tokenId); | ||
| return Number(tokenId); | ||
| }) | ||
| ); | ||
|
|
||
| if (data.nextPageKey) { | ||
| return recursiveFetch(goldenTokens, data.nextPageKey); | ||
| } | ||
| } catch (ex) { | ||
| console.log("error fetching golden tokens", ex); | ||
| } | ||
|
|
||
| return goldenTokens; | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor data processing and pagination logic
- Add a check for
data.nftsbefore mapping. - Remove unnecessary
JSON.parse. - Consider refactoring to an iterative approach for better efficiency and to avoid potential stack overflow.
Apply this diff to address the immediate issues:
const data = await response.json();
goldenTokens = goldenTokens.concat(
- data?.nfts?.map((goldenToken: any) => {
+ (data?.nfts ?? []).map((goldenToken: any) => {
- const tokenId = JSON.parse(goldenToken.tokenId);
- return Number(tokenId);
+ return Number(goldenToken.tokenId);
})
);
if (data.nextPageKey) {
return recursiveFetch(goldenTokens, data.nextPageKey);
}Consider refactoring the entire function to use an iterative approach:
const fetchAllPages = async (): Promise<number[]> => {
let goldenTokens: number[] = [];
let nextPageKey: string | null = null;
do {
// Construct URL (with proper encoding)
try {
// Fetch and process data
// Update nextPageKey
} catch (ex) {
console.error("Error fetching golden tokens:", ex);
break;
}
} while (nextPageKey);
return goldenTokens;
};| try { | ||
| const response = await fetch(url, { | ||
| method: "GET", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); | ||
|
|
There was a problem hiding this comment.
Improve error handling in the fetch request
- Add a response status check before parsing JSON.
- Enhance error handling to provide more context.
Apply this diff to improve error handling:
try {
const response = await fetch(url, {
method: "GET",
headers: {
"Content-Type": "application/json",
},
});
+ if (!response.ok) {
+ throw new Error(`HTTP error! Status: ${response.status}`);
+ }
const data = await response.json();
// ... rest of the code
} catch (ex) {
- console.log("error fetching golden tokens", ex);
+ console.error("Error fetching golden tokens:", ex);
+ throw new Error(`Failed to fetch golden tokens: ${ex.message}`);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| }); | |
| try { | |
| const response = await fetch(url, { | |
| method: "GET", | |
| headers: { | |
| "Content-Type": "application/json", | |
| }, | |
| }); | |
| if (!response.ok) { | |
| throw new Error(`HTTP error! Status: ${response.status}`); | |
| } | |
| const data = await response.json(); | |
| // ... rest of the code | |
| } catch (ex) { | |
| console.error("Error fetching golden tokens:", ex); | |
| throw new Error(`Failed to fetch golden tokens: ${ex.message}`); | |
| } |
| const recursiveFetch: any = async ( | ||
| goldenTokens: any[], | ||
| nextPageKey: string | null | ||
| ) => { | ||
| let url = `${ | ||
| networkConfig[network!].blastUrl | ||
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${indexAddress( | ||
| owner | ||
| ).toLowerCase()}&pageSize=100`; | ||
|
|
||
| if (nextPageKey) { | ||
| url += `&pageKey=${nextPageKey}`; | ||
| } |
There was a problem hiding this comment.
Improve type safety and security in URL construction
- Replace
anytypes with explicit types for better type safety. - Add a runtime check for
networkinstead of using non-null assertion. - URL-encode query parameters to prevent injection attacks.
Apply this diff to address these issues:
- const recursiveFetch: any = async (
- goldenTokens: any[],
+ const recursiveFetch = async (
+ goldenTokens: number[],
nextPageKey: string | null
- ) => {
+ ): Promise<number[]> => {
+ if (!network) {
+ throw new Error("Network is not defined");
+ }
let url = `${
- networkConfig[network!].blastUrl
+ networkConfig[network].blastUrl
- }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${indexAddress(
- owner
- ).toLowerCase()}&pageSize=100`;
+ }/builder/getWalletNFTs?contractAddress=${encodeURIComponent(goldenTokenAddress)}&walletAddress=${encodeURIComponent(indexAddress(owner).toLowerCase())}&pageSize=100`;
if (nextPageKey) {
- url += `&pageKey=${nextPageKey}`;
+ url += `&pageKey=${encodeURIComponent(nextPageKey)}`;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const recursiveFetch: any = async ( | |
| goldenTokens: any[], | |
| nextPageKey: string | null | |
| ) => { | |
| let url = `${ | |
| networkConfig[network!].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${goldenTokenAddress}&walletAddress=${indexAddress( | |
| owner | |
| ).toLowerCase()}&pageSize=100`; | |
| if (nextPageKey) { | |
| url += `&pageKey=${nextPageKey}`; | |
| } | |
| const recursiveFetch = async ( | |
| goldenTokens: number[], | |
| nextPageKey: string | null | |
| ): Promise<number[]> => { | |
| if (!network) { | |
| throw new Error("Network is not defined"); | |
| } | |
| let url = `${ | |
| networkConfig[network].blastUrl | |
| }/builder/getWalletNFTs?contractAddress=${encodeURIComponent(goldenTokenAddress)}&walletAddress=${encodeURIComponent(indexAddress(owner).toLowerCase())}&pageSize=100`; | |
| if (nextPageKey) { | |
| url += `&pageKey=${encodeURIComponent(nextPageKey)}`; | |
| } |
Instead of the old realms world indexing infra we are now using blast apis development api.
Summary by CodeRabbit
New Features
Changes to Existing Features
goldenTokensproperty instead of removed properties.Bug Fixes
Chores